-
Notifications
You must be signed in to change notification settings - Fork 619
[Dashboard] Fix ERC20 claim conditions #5284
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Conversation
|
The latest updates on your projects. Learn more about Vercel for Git ↗︎
|
|
Your org has enabled the Graphite merge queue for merging into mainAdd the label “merge-queue” to the PR and Graphite will automatically add it to the merge queue when it’s ready to merge. Or use the label “hotfix” to add to the merge queue as a hot fix. You must have a Graphite account and log in to Graphite in order to use the merge queue. Sign up using this link. |
|
This stack of pull requests is managed by Graphite. Learn more about stacking. |
size-limit report 📦
|
7c07fda to
047c948
Compare
047c948 to
99c3a53
Compare
Codecov ReportAll modified and coverable lines are covered by tests ✅
Additional details and impacted files@@ Coverage Diff @@
## main #5284 +/- ##
=======================================
Coverage 45.61% 45.61%
=======================================
Files 1068 1068
Lines 55556 55556
Branches 4047 4047
=======================================
Hits 25341 25341
Misses 29525 29525
Partials 690 690
*This pull request uses carry forward flags. Click here to find out more. |
99c3a53 to
867f277
Compare
867f277 to
7f09c4b
Compare
| return "unlimited"; | ||
| } | ||
| if (decimals) { | ||
| return toTokens(value, decimals); |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
why do you need this here? unlimited should be the same no matter the decimals no?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This is similar to toBigInt. While toBigInt converts the value to send it to the blockchain, toUnlimited converts the value from the blockchain to display it on the UI.
Without this new logic, a maxClaimableSupply of 100 tokens will be displayed on the frontend as: 100000000000000000000n
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Updated the code pattern to make it similar to toBigInt
| // so we need to convert it using toUnits | ||
| maxClaimableSupply: toBigInt( | ||
| phase.maxClaimableSupply, | ||
| baseOptions.type === "erc20" ? baseOptions.decimals : undefined, |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
feels a bit confusing to bake this logic inside toBigInt - i think i would separate it at this level, and do an explicit to toUnits here instead
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Ok
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Updated ^
7f09c4b to
1f3ee9c
Compare
Merge activity
|
Fix various issues with ERC20 claim conditions since we up'd it to v5 on the dashboard
<!-- start pr-codex -->
---
## PR-Codex overview
This PR focuses on enhancing the handling of `tokenDecimals` for ERC20 tokens in the claim conditions form, ensuring that the application correctly manages and displays the maximum claimable supply and per wallet limits based on the token's decimal precision.
### Detailed summary
- Updated `MaxClaimableSupplyInput` and `MaxClaimablePerWalletInput` to disable inputs if `tokenDecimals` is not available.
- Changed `tokenDecimals` type from `number` to `number | undefined`.
- Enhanced fetching logic for `tokenDecimals` in the claim conditions form.
- Added error handling for missing `tokenDecimals` when submitting forms.
- Introduced conversion functions for handling ERC20 values in wei and display format.
- Updated `toBigInt` function to return `bigint | undefined`.
- Adjusted `toUnlimited` function to maintain clarity on unlimited values.
> ✨ Ask PR-Codex anything about this PR by commenting with `/codex {your question}`
<!-- end pr-codex -->
1f3ee9c to
d65387b
Compare

Fix various issues with ERC20 claim conditions since we up'd it to v5 on the dashboard
PR-Codex overview
This PR focuses on enhancing the handling of
tokenDecimalsfor ERC20 tokens in theClaimConditionsFormand related components, ensuring proper validation and conversion of values based on token type.Detailed summary
isDisabledproperties inMaxClaimableSupplyInputandMaxClaimablePerWalletInputto account fortokenDecimals.ClaimConditionsFormto handletokenDecimalsasnumber | undefined.tokenDecimals.setClaimPhasesTxandgetClaimPhasesInLegacyFormatto usetokenDecimalsfor ERC20 claims.tokenDecimalscannot be fetched.